Skip to content

refactor(iroh)!: rework net_report#3314

Merged
dignifiedquire merged 79 commits intomainfrom
refactor-address-discovery
Jun 24, 2025
Merged

refactor(iroh)!: rework net_report#3314
dignifiedquire merged 79 commits intomainfrom
refactor-address-discovery

Conversation

@dignifiedquire
Copy link
Copy Markdown
Contributor

@dignifiedquire dignifiedquire commented May 16, 2025 •
edited
Loading

Description

  • Use more Watchable in more places
  • Drops STUN, ICMP and hairpin probes, they are not actually needed in our world
  • restructure report generation
  • Keep QAD connections open, instead of recreating them on every report
  • Removes STUN functionality from the relay server

Fixes

Breaking Changes

  • removes iroh_relay::protos::stun::StunError
  • removes iroh_relay::server::testing::stun_config
  • removes iroh_relay::protos::stun
  • removes iroh_relay::quic::QuicClient::get_addr_and_latency
  • removes DEFAULT_STUN_PORT

Depends on

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 16, 2025 •
edited
Loading

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 88ae171

@n0bot n0bot Bot added this to iroh May 16, 2025
@github-project-automation github-project-automation Bot moved this to 🏗 In progress in iroh May 16, 2025
@dignifiedquire dignifiedquire force-pushed the refactor-address-discovery branch 3 times, most recently from 4ddd06b to 7f985bb Compare May 19, 2025 17:36
flub
flub reviewed May 20, 2025
View reviewed changes
Comment thread iroh/src/net_report/ip_mapped_addrs.rs
Copy link
Copy Markdown
Contributor

@flub flub May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ the visibility qualifiers

@dignifiedquire dignifiedquire changed the title [WIP]: reworkt net_report [WIP]: rework net_report May 20, 2025
@dignifiedquire dignifiedquire force-pushed the refactor-address-discovery branch 2 times, most recently from bb2c8fb to 0fddf40 Compare May 23, 2025 12:00
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 23, 2025 •
edited
Loading

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3314/docs/iroh/

Last updated: 2025-06-24T13:03:46Z

@dignifiedquire dignifiedquire force-pushed the refactor-address-discovery branch from f9bacd4 to e9e7bc3 Compare May 27, 2025 08:57
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 27, 2025

refactor-address-discovery.4886027
Perf report:

test case throughput_gbps throughput_transfer
iroh_cust_10gb 1_to_1 2.26 2.86
iroh_cust_10gb 1_to_3 6.11 7.54
iroh_cust_10gb 1_to_5 10.61 13.23
iroh_cust_10gb 1_to_10 14.79 17.16
iroh_cust_10gb 2_to_2 4.26 5.32
iroh_cust_10gb 2_to_4 7.13 8.56
iroh_cust_10gb 2_to_6 10.30 12.26
iroh_cust_10gb 2_to_10 15.94 18.72
iroh_latency_200ms 1_to_1 0.72 2.16
iroh_latency_200ms 1_to_3 2.25 7.45
iroh_latency_200ms 1_to_5 3.63 11.26
iroh_latency_200ms 1_to_10 6.64 17.51
iroh_latency_200ms 2_to_2 1.42 4.19
iroh_latency_200ms 2_to_4 2.95 9.48
iroh_latency_200ms 2_to_6 4.35 13.42
iroh_latency_200ms 2_to_10 7.14 21.40
iroh_latency_20ms 1_to_1 0.74 2.38
iroh_latency_20ms 1_to_3 2.25 7.48
iroh_latency_20ms 1_to_5 3.65 11.40
iroh_latency_20ms 1_to_10 6.51 16.57
iroh_latency_20ms 2_to_2 1.43 4.28
iroh_latency_20ms 2_to_4 2.95 9.43
iroh_latency_20ms 2_to_6 4.40 13.90
iroh_latency_20ms 2_to_10 7.27 22.66
iroh 1_to_1 0.72 2.19
iroh 1_to_3 2.30 8.11
iroh 1_to_5 3.66 11.53
iroh 1_to_10 6.58 17.03
iroh 2_to_2 1.41 4.12
iroh 2_to_4 2.94 9.39
iroh 2_to_6 4.40 13.91
iroh 2_to_10 7.19 21.87
iroh_relay_only 1_to_1 2.94 2.96
iroh_relay_only 1_to_3 4.65 4.66
iroh_relay_only 1_to_5 4.69 4.70
iroh_relay_only 1_to_10 4.37 4.38
iroh_relay_only 2_to_2 3.86 3.88
iroh_relay_only 2_to_4 6.67 6.69
iroh_relay_only 2_to_6 7.11 7.14
iroh_relay_only 2_to_10 8.67 8.70

@dignifiedquire dignifiedquire force-pushed the refactor-address-discovery branch 7 times, most recently from e807b2b to d4f3740 Compare June 3, 2025 18:01
@dignifiedquire dignifiedquire force-pushed the refactor-address-discovery branch 2 times, most recently from a0c876d to 3610be3 Compare June 5, 2025 11:17
@dignifiedquire dignifiedquire marked this pull request as ready for review June 5, 2025 11:17
@dignifiedquire dignifiedquire changed the title [WIP]: rework net_report refactor(iroh): rework net_report Jun 5, 2025
ramfox
ramfox reviewed Jun 5, 2025
View reviewed changes
Comment thread iroh-relay/src/main.rs Outdated
ramfox
ramfox reviewed Jun 5, 2025
View reviewed changes
Comment thread iroh/src/net_report/probes.rs Outdated
ramfox
ramfox approved these changes Jun 9, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ramfox ramfox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_until_cancelled_owned, reducing the probes, and cleaning up to net_report::Client makes this much more easy to reason about.

I'd argue we still have a bit of cleanup around the probe plan creation, but this is a long-needed change!

Comment thread iroh/src/net_report/report.rs Outdated
Comment thread iroh/src/net_report/probes.rs Outdated
Comment thread iroh/src/net_report.rs
@dignifiedquire dignifiedquire force-pushed the refactor-address-discovery branch 2 times, most recently from d09c878 to 85e6939 Compare June 11, 2025 14:07
@dignifiedquire dignifiedquire changed the base branch from refactor-transports to main June 11, 2025 14:13
@dignifiedquire dignifiedquire force-pushed the refactor-address-discovery branch from 303dce9 to 542361e Compare June 23, 2025 12:52
@dignifiedquire dignifiedquire mentioned this pull request Jun 23, 2025
Merged
@dignifiedquire
Copy link
Copy Markdown
Contributor Author

dignifiedquire commented Jun 23, 2025

If we never get any responses in probe_tx in the loop + tokio::select!, we don't seem to ever run self.have_enough_reports. This means that setting the timeout_fut never happens. The timeout_fut arm of tokio::select! is the only arm where we break theget_report loop.

thanks for checking this, the reason we exit is because reporgen::Actor gets dropped, and the receiver returns None where we break out of the loop

@dignifiedquire
Copy link
Copy Markdown
Contributor Author

dignifiedquire commented Jun 23, 2025

For the maintained connections, we keep around the first qad conn that comes back with a response. I can't imagine how this wouldn't be the one with the lowest latency, but since we only rely on latency to determine with relay we consider the preferred_relay, couldn't it techincally be possible for us to recommend a different relay than the one we are connected to? Should we refer to the QadConn relays first when trying to determine the preferred relay, before relaying on latencies if the QadConns don't exist?

Hmm, it is certainly possible for the two to differ, and I could see us optimizing the differences in the future, but for now I think this should "just work out", as the latencies we will get from these QADs will be the ones returned in every run, and subsequently used for evaluation

@dignifiedquire dignifiedquire changed the title refactor(iroh): rework net_report refactor(iroh)!: rework net_report Jun 24, 2025
@dignifiedquire dignifiedquire added this pull request to the merge queue Jun 24, 2025
Merged via the queue into main with commit dcbebe9 Jun 24, 2025
28 of 29 checks passed
@github-project-automation github-project-automation Bot moved this from 🏗 In progress to ✅ Done in iroh Jun 24, 2025
@ramfox ramfox deleted the refactor-address-discovery branch June 25, 2025 17:17
Arqu added a commit to n0-computer/iroh-doctor that referenced this pull request Jun 25, 2025
@dignifiedquire @Arqu
feat: update to be used with the net-report refactor (#36)
ec81da0 
ref n0-computer/iroh#3314

---------

Co-authored-by: Asmir Avdicevic <asmir.avdicevic64@gmail.com>
@ramfox ramfox mentioned this pull request Sep 24, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Frando Frando Frando left review comments

@flub flub flub left review comments

@ramfox ramfox ramfox approved these changes

Assignees

No one assigned

Labels

None yet

Projects

iroh
Status: ✅ Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dignifiedquire @ramfox @Frando @flub